-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pickling support for arrays, buffers #786
base: main
Are you sure you want to change the base?
Conversation
pyopencl/array.py
Outdated
if self.allocator is None: | ||
context = queue.context | ||
self.base_data = cl.Buffer(context, cl.mem_flags.READ_WRITE, self.nbytes) | ||
else: | ||
self.base_data = self.allocator(self.nbytes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this would allocate redundantly if there were multiple arrays using the same buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, do you have a suggestion on how to handle this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b = a[:]
will make another array using the same buffer. (Any slice really.)
A way around this would be to make buffers picklable instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 490bb94 going in the direction you had in mind? Should this be a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it does. I'm just not clear on why we need two separate queue_for_pickling
decorators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just not clear on why we need two separate
queue_for_pickling
decorators.
60e71ee removes the duplicate.
pyopencl/array.py
Outdated
if self.allocator is None: | ||
context = queue.context | ||
self.base_data = cl.Buffer(context, cl.mem_flags.READ_WRITE, self.nbytes) | ||
else: | ||
self.base_data = self.allocator(self.nbytes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to allow the user to supply an allocator via the context manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of 25ebc7b?
test/test_array.py
Outdated
@@ -2393,6 +2393,26 @@ def test_xdg_cache_home(ctx_factory): | |||
# }}} | |||
|
|||
|
|||
# {{{ test pickling | |||
|
|||
def test_array_pickling(ctx_factory): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test that this works for SVM-backed arrays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done as part of 25ebc7b
@@ -705,6 +736,63 @@ def __init__( | |||
"than expected, potentially leading to crashes.", | |||
InconsistentOpenCLQueueWarning, stacklevel=2) | |||
|
|||
# {{{ Pickling | |||
|
|||
def __getstate__(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be useful if this worked for subclasses (liked TaggedCLArray
), too. Should be tested, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of fdb3525 ?
Unsubscribing... @-mention or request review once it's ready for a look or needs attention. |
3713741
to
498d7e2
Compare
283faca
to
eab5edc
Compare
Co-authored-by: Andreas Klöckner <[email protected]>
0ec19de
to
00876e1
Compare
pooled allocations: look at using |
Based on the
meshmode.DOFArray
pickling support.TODOs:
Please squash